Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LCFS - Implement Email Notification Triggers in Backend for Subscribed Users #1226 #1402

Merged
merged 20 commits into from
Dec 12, 2024

Conversation

prv-proton
Copy link
Collaborator

Copy link

github-actions bot commented Dec 9, 2024

Backend Test Results

493 tests  ±0   491 ✅ ±0   1m 52s ⏱️ +4s
  1 suites ±0     0 💤 ±0 
  1 files   ±0     2 ❌ ±0 

For more details on these failures, see this check.

Results for commit c77f4c6. ± Comparison against base commit 3c014c2.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Dec 9, 2024

Frontend Test Results

  1 files  ±0  116 suites  ±0   38s ⏱️ ±0s
390 tests ±0  370 ✅ ±0  20 💤 ±0  0 ❌ ±0 
392 runs  ±0  372 ✅ ±0  20 💤 ±0  0 ❌ ±0 

Results for commit c77f4c6. ± Comparison against base commit 3c014c2.

♻️ This comment has been updated with latest results.

@prv-proton prv-proton marked this pull request as draft December 9, 2024 17:53
@prv-proton prv-proton marked this pull request as ready for review December 9, 2024 20:37
@prv-proton prv-proton requested review from hamed-valiollahi and kevin-hashimoto and removed request for hamed-valiollahi December 10, 2024 19:22
Copy link
Collaborator

@AlexZorkin AlexZorkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good, just one minor question. 👍

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a better place to keep this in the backend? Or is it used in enough places to justify having it in base?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Alex for the review. Since this schema will be used across various services, it causes greenlet spawn error if placed inside the notifications schema, hence it's being placed inside the base.

@prv-proton prv-proton merged commit a583a79 into release-0.2.0 Dec 12, 2024
10 checks passed
@prv-proton prv-proton deleted the feat/prashanth-email-trigger-1226 branch December 12, 2024 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants